Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicit visibility for function load #124

Merged
merged 1 commit into from
Dec 2, 2013
Merged

Add explicit visibility for function load #124

merged 1 commit into from
Dec 2, 2013

Conversation

tlfbrito
Copy link

@tlfbrito tlfbrito commented Dec 1, 2013

Explicit modifier scope for function load

Explicit modifier scope for function load
stof added a commit that referenced this pull request Dec 2, 2013
Add explicit visibility for function load
@stof stof merged commit 4a81be0 into doctrine:master Dec 2, 2013
@guilhermeblanco
Copy link
Member

@stof why was this merged? There's no way of having an interface that enforces a non-public method. It makes no sense to me.

@Ocramius
Copy link
Member

Ocramius commented Dec 3, 2013

@guilhermeblanco consistency

@guilhermeblanco
Copy link
Member

@Ocramius still doesn't make sense... I'm gonna quote PHP.net: http://php.net/interface

All methods declared in an interface must be public, this is the nature of an interface.

I just don't see the need of adding public. Even PHP manual does that... a bit weird to me.

@Ocramius
Copy link
Member

Ocramius commented Dec 3, 2013

It's just for the sake of having visibility everywhere all the time

@texdc
Copy link

texdc commented Dec 3, 2013

It's actually a CS issue. I'd rather not put public all over my interfaces, but CS checkers are whiny about that.

@tlfbrito
Copy link
Author

tlfbrito commented Dec 3, 2013

@guilhermeblanco Check that even in the php.net interface example have the explicity visibility as public defined.
This also help IDEs generating the implementation methods with the right visibility and this is required by PSR-2.

@stof
Copy link
Member

stof commented Dec 4, 2013

@guilhermeblanco Following PSR-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants